Skip to content

fix(core): enforce maxTotalTimeout as a hard cap - #3155

Draft
Sanath91009 wants to merge 1 commit into
modelcontextprotocol:mainfrom
Sanath91009:fix/max-total-timeout-enforced
Draft

Sanath91009 wants to merge 1 commit into
modelcontextprotocol:mainfrom
Sanath91009:fix/max-total-timeout-enforced

Conversation

@Sanath91009

@Sanath91009 Sanath91009 commented Nov 6, 2026 •

Copy link
Copy Markdown

Fixes #2695.

maxTotalTimeout is documented as a hard cap, but _setupTimeout armed the pending timer with timeout only. The cap was read in _resetTimeout, which runs only when a notifications/progress arrives with resetTimeoutOnProgress set. A request that never sends progress therefore outlived its own deadline, and a progress reset re-armed the full timeout instead of what was left of the total budget.

Reproduction

On main at 1e608ca, TestProtocol over a MockTransport that swallows the request:

testRequest(protocol, request, schema, { timeout: 1000, maxTotalTimeout: 150 });
  • At 151ms the request is still pending. It rejects at 1000ms with Request timed out; the 150ms cap did nothing.
  • With { timeout: 1000, maxTotalTimeout: 300, resetTimeoutOnProgress: true } and one progress notification at 200ms, the timer is re-armed for the full 1000ms, so the request settles at 1200ms instead of 300ms.

Change

  • _setupTimeout arms the timer with min(timeout, remaining maxTotalTimeout).
  • _resetTimeout re-arms with the remaining total budget rather than the full timeout.
  • The timer reports which budget fired, so a cap expiry rejects with Maximum total timeout exceeded and { maxTotalTimeout, totalElapsed } instead of Request timed out.

inputRequiredDriver passes a shrinking remaining budget back in as maxTotalTimeout, so it gets the same bound per leg.

Tests

Three tests added to packages/core-internal/test/shared/protocol.test.ts:

  • should enforce maxTotalTimeout when no progress notification arrives — fails on main, passes with the fix.
  • should re-arm the timeout within the remaining maxTotalTimeout budget — fails on main, passes with the fix.
  • should still report the per-request timeout when it is shorter than maxTotalTimeout — guard, passes both ways.

Verified by reverting only protocol.ts: 2 failed, 2 passed. With the fix: 4 passed.

Full suites green: packages/core-internal 78 files / 1853 tests, packages/server 55 files / 674 tests. tsgo --noEmit, ESLint and Prettier clean. Changeset added.

Note on #2763

#2763 covers the same issue and has been conflicted against main since 2026-09-23. I did not want to add a third duplicate, but a PR that cannot merge does not fix the bug. Happy to close this one if @claxman rebases and it lands first.

_setupTimeout armed the pending timer with `timeout` only, so
`maxTotalTimeout` was read solely in `_resetTimeout`, which runs only on a
progress notification with `resetTimeoutOnProgress`. A request that never
sends progress outlived its documented hard cap, and a progress reset
re-armed the full `timeout` instead of the remaining total budget.

Arm the timer with min(timeout, remaining maxTotalTimeout), re-arm with the
remaining budget on reset, and report `Maximum total timeout exceeded` when
the cap is what fired.
@changeset-bot

changeset-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5e065c8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/client Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/core-internal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Nov 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3155

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3155

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3155

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3155

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3155

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3155

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3155

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3155

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3155

commit: 5e065c8

@claude claude Bot added the v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes label Nov 6, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

maxTotalTimeout is never enforced unless a progress notification arrives

1 participant